Conversation
📝 WalkthroughWalkthroughUpdates the Transcript Viewer’s activity logic to be session-specific, determining inactivity per session using ongoingSessionId and ongoingStatus. Adjusts the Tab Header’s recording indicator visuals by replacing a single pulsing dot with a composed animated indicator. No data model changes; rendering and conditional UI logic are updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant TranscriptViewer
participant SessionState as OngoingSession State
participant UI as SpeakerSelector UI
User->>TranscriptViewer: Open Transcript (sessionId)
TranscriptViewer->>SessionState: Get ongoingSessionId, ongoingStatus
Note over TranscriptViewer: Compute isThisSessionActive = (sessionId == ongoingSessionId) && (ongoingStatus == "running_active")
alt Session inactive (per-session)
TranscriptViewer->>UI: Render speaker selector (inactive = true)
else Session active
TranscriptViewer-->>User: Skip inactive UI (early return path)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/components/editor-area/transcript-viewer.tsx (1)
185-189: Add explicit guard for valid session IDs in the comparison.The comparison
ongoingSessionId === sessionIdon line 188 could incorrectly evaluate totruewhen both values areundefined(e.g., no ongoing session and invalid route). While line 216 guards against!sessionId, making the logic more explicit would prevent potential edge-case bugs if the guard is moved or removed.Consider this more defensive approach:
- // Check if THIS SPECIFIC session is inactive, not global status - const ongoingSessionId = useOngoingSession(s => s.sessionId); - const ongoingStatus = useOngoingSession(s => s.status); - const isThisSessionActive = ongoingStatus === "running_active" && ongoingSessionId === sessionId; - const inactive = !isThisSessionActive; + // Check if THIS SPECIFIC session is inactive, not global status + const ongoingSessionId = useOngoingSession(s => s.sessionId); + const ongoingStatus = useOngoingSession(s => s.status); + const isThisSessionActive = + sessionId != null && + ongoingSessionId === sessionId && + ongoingStatus === "running_active"; + const inactive = !isThisSessionActive;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/editor-area/note-header/tab-header.tsx(1 hunks)apps/desktop/src/components/editor-area/transcript-viewer.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/editor-area/note-header/tab-header.tsxapps/desktop/src/components/editor-area/transcript-viewer.tsx
🧬 Code graph analysis (1)
apps/desktop/src/components/editor-area/transcript-viewer.tsx (1)
packages/utils/src/contexts/ongoing-session.tsx (1)
useOngoingSession(32-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (1)
apps/desktop/src/components/editor-area/note-header/tab-header.tsx (1)
129-134: LGTM! Clean visual upgrade for the recording indicator.The new composed indicator with a semi-transparent outer circle and pulsing inner circle provides better visual feedback than the previous single pulsing dot. Both circles correctly use
absolute inset-0to fill the parent container, creating the intended layered effect.
DON'T MERGE THIS YET